Skip to content

fix bug in check for valid URL - #317

Merged
Schiano-NOAA merged 1 commit into
nmfs-ost:mainfrom
iantaylor-NOAA:main
Jul 30, 2026
Merged

fix bug in check for valid URL#317
Schiano-NOAA merged 1 commit into
nmfs-ost:mainfrom
iantaylor-NOAA:main

Conversation

@iantaylor-NOAA

Copy link
Copy Markdown
Contributor

Long description of a simple fix:

The changes to convert_output() in #305 seem to include a copilot error where curlGetHeaders(file) is associated with the utils package when it should just be base R.

After getting an Invalid URL. error in a FIMS case study (https://github.com/NOAA-FIMS/case-studies/actions/runs/30038020469/job/89311014050#step:7:26), I tracked down the problem to this line. Because it's wrapped in tryCatch(), R can't tell the difference between an invalid URL and a function that doesn't exist, but running outside tryCatch() makes it clear which one works:

r$> utils::curlGetHeaders("https://raw.githubusercontent.com/r4ss/r4ss/refs/heads/main/inst/extdata/simple_small/Report.sso")
Error: 'curlGetHeaders' is not an exported object from 'namespace:utils'

r$> curlGetHeaders("https://raw.githubusercontent.com/r4ss/r4ss/refs/heads/main/inst/extdata/simple_small/Report.sso")
 [1] "HTTP/2 200 \r\n"                                                                    
 [2] "cache-control: max-age=300\r\n"                                                     
 [3] "content-security-policy: default-src 'none'; style-src 'unsafe-inline'; sandbox\r\n"
...

I thought of trying to add an additional testthat test to confirm that convert_output() will work with a valid URL, but the URL above as well as https://raw.githubusercontent.com/pfmc-assessments/petrale/main/models/2023.a050.003_FIMS_case-study_wtatage/Report.sso are not being converted correctly right now, so are not well suited to automated testing.

@Schiano-NOAA

Copy link
Copy Markdown
Collaborator

Thank you so much Ian for identifying this issue and providing a fix! I am going to run a few checks locally then merge in.

@Schiano-NOAA
Schiano-NOAA merged commit 2814744 into nmfs-ost:main Jul 30, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants